feat(analytics-controller): add optional context parameter for track, identify, view#8835
Conversation
gauthierpetetin
changed the title
gauthierpetetin
changed the title
gauthierpetetin
force-pushed
the
codex/analytics-controller-context-options
branch
from
d508c40 to
f3dcf3e
Compare
gauthierpetetin
temporarily deployed
to
default-branch
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f3dcf3e. Configure here.
| this.#platformAdapter.track(event.name, undefined, context); | ||
| } else { | ||
| this.#platformAdapter.track(event.name); | ||
| } |
There was a problem hiding this comment.
Redundant context branching repeated five times
Low Severity
The if (context) { call(args, context) } else { call(args) } branching pattern is duplicated five times across trackEvent, identify, and trackView. Since context is typed as optional (context?: AnalyticsContext) on the AnalyticsPlatformAdapter methods, simply always passing context (which is undefined when omitted) achieves the same result in a single call per site, removing ~20 lines of redundant branching and reducing the maintenance surface for future changes.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f3dcf3e. Configure here.
NicolasMassart
left a comment
NicolasMassart
left a comment
There was a problem hiding this comment.
LGTM. The optional analytics context is threaded through trackEvent, identify, and trackView while preserving the existing omitted-context call behavior. The added coverage exercises the main forwarding paths, including no-property events and split anonymous events.
2 participants
Explanation
Add the possibility to forward context to AnalyticsController for trackEvent, identify, and trackView functions, as this is needed in Extension.
Validation
yarn workspace @metamask/analytics-controller run jest --no-coverage packages/analytics-controller/src/AnalyticsController.test.tsyarn workspace @metamask/analytics-controller run messenger-action-types:checkyarn workspace @metamask/analytics-controller run changelog:validateyarn workspace @metamask/analytics-controller run buildyarn eslint packages/analytics-controller/src/AnalyticsController.ts packages/analytics-controller/src/AnalyticsController.test.ts packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts packages/analytics-controller/src/AnalyticsController-method-action-types.ts packages/analytics-controller/src/index.tsReferences
Replaces #8701 which is no longer needed
Note
Low Risk
Low risk: adds an optional
contextpayload and forwards it to adapters without changing existing behavior when omitted; main risk is downstream TypeScript/interface compatibility forAnalyticsPlatformAdapterimplementations.Overview
Adds an optional
AnalyticsContextparameter toAnalyticsControllermethodstrackEvent,identify, andtrackView, and forwards it through to the underlyingAnalyticsPlatformAdapter(track/identify/view).Updates the adapter type definitions and public exports to include
AnalyticsContext, and extends unit tests plus changelog to cover context forwarding (including event-splitting behavior and events without properties).Reviewed by Cursor Bugbot for commit f3dcf3e. Bugbot is set up for automated code reviews on this repo. Configure here.